[prometheus] Eliminate labels fingerprint#9785
Conversation
…etheus_fingerprint
ObservationData collection with TSDB enabled: Found 1 doc in the discover
Data collection without TSDB enabled: Found 64 docs in the discover
Both setup having the same instance with same configurations. In the TSDB disabled setup, all the 64 docs are having different prometheus.labels.* field with same timestamp. It seems like the valid documents are dropping by enabling TSDB! |
|
Thanks @harnish-elastic for validating. For changes involving TSDB, especially when the dimension fields are removed, please run the TSDB test toolkit and include it in the PR. It helps to detect if any documents are dropped. |
@agithomas, I have also run the TSDB test toolkit, the observation is as below, Run test toolkit without TSDB enabled in Kibana 8.13.0, OutputValues being used: elasticsearch_host = https://localhost:9200 elasticsearch_ca_path = /root/.elastic-package/profiles/default/certs/elasticsearch/ca-cert.pem elasticsearch_user = elastic elasticsearch_pwd = changeme data_stream = metrics-prometheus.collector-default docs_index = 0 settings_mappings_index = Last index of the data stream max_docs = All documents get_overlapping_files = True directory_overlapping_files = overwritten-docs-metrics-prometheus.collector-default display_docs = 10 copy_docs_per_dimension = 2You're testing with version 8.13.0. Testing data stream metrics-prometheus.collector-default. Routing path is empty. Program will end. |
|
When using stack version {
"prometheus.labels.*": {
"path_match": "prometheus.labels.*",
"match_mapping_type": "string",
"mapping": {
"type": "keyword"
}
}
},Dynamic mappings for {
"prometheus.labels.*": {
"path_match": "prometheus.labels.*",
"match_mapping_type": "string",
"mapping": {
"time_series_dimension": true,
"type": "keyword"
}
}
},TSDB test: Each label is being recognized as a dimension so it seems fine. @agithomas WDYT? |
|
From the TSDB toolkit output, there seems to be no drop in the documents. |
|
@agithomas if the dynamic mapping of labels is not making each label a dimension in |
@felixbarny ,can you confirm if the feature is available from 8.14.0 or 8.13.0. When tested with 8.13.0, we observed documents getting dropped, possibly indicating missing feature / functionality. [Reference]. (#9785 (comment)) . |
|
Tested TSDB enablement with 8.14.0-SNAPSHOT, TSDB works fine in 8.14.0-SNAPSHOT. I can see 140 events in before TSDB and after TSDB.
MappingsValues being used: elasticsearch_host = https://localhost:9200 elasticsearch_ca_path = /root/.elastic-package/profiles/default/certs/elasticsearch/ca-cert.pem elasticsearch_user = elastic elasticsearch_pwd = changeme data_stream = metrics-prometheus.collector-default docs_index = 0 settings_mappings_index = Last index of the data stream max_docs = All documents get_overlapping_files = True directory_overlapping_files = overwritten-docs-metrics-prometheus.collector-default display_docs = 10 copy_docs_per_dimension = 2You're testing with version 8.14.0-SNAPSHOT. Testing data stream metrics-prometheus.collector-default. The time series fields for the TSDB index are: Index tsdb-index-enabled successfully created. Copying documents from .ds-metrics-prometheus.collector-default-2024.05.15-000001 to tsdb-index-enabled... |
packages/prometheus/manifest.yml
Outdated
| - containers | ||
| conditions: | ||
| kibana.version: "^8.12.1" | ||
| kibana.version: "^8.13.0" |
There was a problem hiding this comment.
Do we need to update this version to 8.14.0? As in 8.13.0 kibana seems like with the TSDB, seems like the events are getting dropped!
🚀 Benchmarks reportTo see the full report comment with |
|
/test |
1 similar comment
|
/test |
|
After bumping to |
gpop63
force-pushed
the
tsdb_eliminate_prometheus_fingerprint
branch
from
0e95b3d to
25446ae
Compare
|
@jsoriano do you have any idea why the kibana container just wont start? The only thing I did is bump kibana version to |
|
@gpop63 there is an issue with current 8.14 snapshots, it will be fixed in next snapshots, probably today. More info in elastic/kibana#183334 (comment). |
|
/test |
|
agithomas
left a comment
agithomas
left a comment
There was a problem hiding this comment.
LGTM, as no document drop reported as part of the included test results.
packages/prometheus/changelog.yml
Outdated
| # newer versions go on top | ||
| - version: "1.15.3" | ||
| changes: | ||
| - description: Eliminate labels fingerprint for tsdb purposes and use keyword type instead. |
There was a problem hiding this comment.
I think, Remove instead of Eliminate would be a better word.
Also, I wonder if it is correct/important to have the phrase - and use the keyword type instead.
agithomas
left a comment
agithomas
left a comment
There was a problem hiding this comment.
Query regarding changelog entry added.
agithomas
dismissed
their stale review
Dismissing the review as certain clarity regarding field removal is needed.
packages/prometheus/data_stream/remote_write/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
…etheus_fingerprint
|
💚 Build Succeeded
History
cc @gpop63 @harnish-elastic |
|
Package prometheus - 1.17.0 containing this change is available at https://epr.elastic.co/search?package=prometheus |
9 participants
Proposed commit message
Since
8.13.08.14.0we can mark labels field as a dimension directly without having to use the fingerprint processor. Issue was fixed in elastic/elasticsearch#93564.Removed entire pipeline for
collectordata stream as it was only being used for fingerprinting.Data streams affected:
collectorqueryremote_writeChecklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots